Fix 4626 classification#4638
Conversation
|
Claude finished @fleupold's task in 2m 33s —— View job PR Review: Fix 4626 classification
Verdict: LGTM ✅Clean, well-scoped fix. The logic correctly distinguishes a contract-level rejection (partial/non-conforming EIP-4626 token) from a transient transport failure:
Minor (non-blocking, pre-existing)The negative cache is process-lifetime and never evicted. A legitimate vault that is temporarily reverting on |
jmg-duarte
left a comment
jmg-duarte
left a comment
There was a problem hiding this comment.
LGTM though we could add an E2E test that uses wmtUSDC, something like:
async fn eip4626_partial_vault_terminal_token_test(web3: Web3) {
// wmtUSDC is expected to classify as non-vault, so `inner`'s price
// round-trips unchanged — any fixed value works.
let expected_price = 0.0001;
let inner = FixedPrice(expected_price);
let estimator = Eip4626::new(Box::new(inner), web3.provider);
let price = estimator
.estimate_native_price(WMT_USDC, HEALTHY_PRICE_ESTIMATION_TIME)
.await
.expect("token missing convertToAssets() must classify as non-vault, not abort");
assert_eq!(price, expected_price);
}
The unit test works just as well for this though, I leave this up to you
2 participants
Description
Wintermute is looking to enable quotes for their Wildcat issued wmtUSDx tokens (e.g. wmtUSDC). Currently those tokens fail quoting because we cannot find a native price. The reason for this is that the token looks like an ERC-4626 vault token (ie. it implements
assets()) but doesn't follow the standard (ie it doesn't implementconvertToAssets()).In our current logic failure to convert gets bubbled up directly as an estimation error (without attempting the standard ERC20 estimator path)
Changes
How to test
Added unit test